Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Vizdataset type #993

Merged
merged 1 commit into from
Jun 10, 2024
Merged

Add Vizdataset type #993

merged 1 commit into from
Jun 10, 2024

Conversation

hanbyul-here
Copy link
Collaborator

@hanbyul-here hanbyul-here commented Jun 7, 2024

This PR sets up the base datatype for visualization, so the types can be used for all the map-related components with the new Map component. (instead of TimelineDataset)

Copy link

netlify bot commented Jun 7, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit 5e236ed
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/66631795e9967e0008b9636b
😎 Deploy Preview https://deploy-preview-993--veda-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hanbyul-here hanbyul-here force-pushed the viz-datatype branch 2 times, most recently from f7d49c8 to fd01b14 Compare June 7, 2024 14:20
settings: TimelineDatasetSettings;
analysis: TimelineDatasetAnalysisIdle;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, what is the goal here? To create a new type to just separate out analysis or to eventually move towards having a clear separation between these Viz & timelinedataset types? If the first, wondering why not something like type VizDatasetIdle = Omit<TimelineDatasetIdle, "analysis"> ?

Copy link
Collaborator Author

@hanbyul-here hanbyul-here Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal is to establish the type with attributes only for visualization (for the components needing map visualization), so we can use TimelineDatasets only for Timeline Datasets. (And all other components that only need visualizations - BlockMap, ScrollyMap, s-explore map can use VizDataset)

I think either the current approach or type VizDatasetIdle = Omit<TimelineDatasetIdle, "analysis"> as you suggested works. But in my mind, VizDataset is a more fundamental block and TimelineDataset is the type derived from it - so VizDataset + analysis = TimelineDataset rather than TimelineDataset - analysis = VizDataset.

@dzole0311
Copy link
Collaborator

Merging this to try and use it for 967

@dzole0311 dzole0311 merged commit aa149b3 into main Jun 10, 2024
8 checks passed
@dzole0311 dzole0311 deleted the viz-datatype branch June 10, 2024 08:40
sandrahoang686 added a commit that referenced this pull request Jun 17, 2024
## What's Changed
Bump minor version as part of release cycle

## 🎉 Features
- Port the upgraded catalog view in the E&A dataset selector modal
#989

## 🚀 Improvements
- Replace legacy Mapbox component for Scrollytelling
#992
- Add Vizdataset type #993

## 🐛 Fixes
🦗
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants